Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor improves, mainly typos and comments #124

Merged
merged 2 commits into from
Feb 12, 2024
Merged

Minor improves, mainly typos and comments #124

merged 2 commits into from
Feb 12, 2024

Conversation

freddy77
Copy link
Collaborator

@freddy77 freddy77 commented Feb 9, 2024 •
edited
Loading

  • Fix minor typos and missing apostrophes;
  • Make too files more similar.

@andyhhp
Copy link
Contributor

andyhhp commented Feb 9, 2024

Looks ok to me, but that Py3.6 unit test failure looks like a real python issue. Probably worth fixing in this cleanup PR too.

@freddy77
Copy link
Collaborator Author

freddy77 commented Feb 9, 2024

Error is pretty weird

2024-02-09T14:57:55.9099513Z Failure. Coverage is below 100%.
2024-02-09T14:57:55.9100518Z -------------
2024-02-09T14:57:55.9100977Z Diff Coverage
2024-02-09T14:57:55.9101402Z Diff: origin/master...HEAD
2024-02-09T14:57:55.9101903Z -------------
2024-02-09T14:57:55.9102430Z xcp/net/ifrename/dynamic.py (0.0%): Missing lines 131
2024-02-09T14:57:55.9103126Z -------------
2024-02-09T14:57:55.9103487Z Total:   1 line
2024-02-09T14:57:55.9103852Z Missing: 1 line
2024-02-09T14:57:55.9104246Z Coverage: 0%
2024-02-09T14:57:55.9104642Z -------------
2024-02-09T14:57:55.9104855Z 
2024-02-09T14:57:55.9105151Z --- xcp/net/ifrename/dynamic.py ---
2024-02-09T14:57:55.9105532Z 
2024-02-09T14:57:55.9106499Z 0001:             LOG.warning(�[33m"�[39;49;00m�[33mDynamic rules appear to be corrupt�[39;49;00m�[33m"�[39;49;00m)�[37m�[39;49;00m
2024-02-09T14:57:55.9108049Z 0002:             �[34mreturn�[39;49;00m �[34mFalse�[39;49;00m�[37m�[39;49;00m
2024-02-09T14:57:55.9109127Z 0003:         �[37m# The installer has no json.�[39;49;00m�[37m�[39;49;00m
2024-02-09T14:57:55.9110255Z 0004:         �[34mexcept�[39;49;00m �[36mNameError�[39;49;00m:�[37m�[39;49;00m
2024-02-09T14:57:55.9112166Z 0005:             LOG.warning(�[33m"�[39;49;00m�[33mModule json not available.  Can�[39;49;00m�[33m'�[39;49;00m�[33mt parse dynamic rules.�[39;49;00m�[33m"�[39;49;00m)�[37m�[39;49;00m
2024-02-09T14:57:55.9113401Z 0006:             �[34mreturn�[39;49;00m �[34mFalse�[39;49;00m�[37m�[39;49;00m
2024-02-09T14:57:55.9113881Z 0007: �[37m�[39;49;00m
2024-02-09T14:57:55.9114539Z 0008:         �[34mif�[39;49;00m �[33m"�[39;49;00m�[33mlastboot�[39;49;00m�[33m"�[39;49;00m �[35min�[39;49;00m info:�[37m�[39;49;00m
2024-02-09T14:57:55.9115730Z 0009:             �[34mfor�[39;49;00m entry �[35min�[39;49;00m info[�[33m"�[39;49;00m�[33mlastboot�[39;49;00m�[33m"�[39;49;00m]:
2024-02-09T14:57:55.9116210Z 
2024-02-09T14:57:55.9116215Z 
2024-02-09T14:57:55.9116220Z 
2024-02-09T14:57:55.9487170Z 1

so... are you not allowed to fix typos in logs to avoid coverage changes?

@freddy77
Fix some typos and missing apostrophes
3d733d3 
Signed-off-by: Frediano Ziglio <[email protected]>
@freddy77
Makes xcp/net/ifrename/{dynamic.py,static.py} more similar
c8ee010 
They are pretty similar, sync some differences.

Signed-off-by: Frediano Ziglio <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Feb 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (feeb478) 83.25% compared to head (c8ee010) 83.25%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   83.25%   83.25%           
=======================================
  Files          22       22           
  Lines        3363     3363           
=======================================
  Hits         2800     2800           
  Misses        563      563
Flag Coverage Δ
unittest 83.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@freddy77
Copy link
Collaborator Author

Removed the line which makes the CI fail.

@andyhhp andyhhp merged commit 21987f1 into xenserver:master Feb 12, 2024
6 checks passed
@freddy77 freddy77 deleted the minor branch February 12, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyhhp andyhhp andyhhp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@freddy77 @andyhhp